Skip to content

JS: add the resolve library as a sink to js/path-injection#6015

Merged
codeql-ci merged 2 commits into
github:mainfrom
erik-krogh:resolve
Jun 8, 2021
Merged

JS: add the resolve library as a sink to js/path-injection#6015
codeql-ci merged 2 commits into
github:mainfrom
erik-krogh:resolve

Conversation

@erik-krogh

@erik-krogh erik-krogh commented Jun 4, 2021

Copy link
Copy Markdown
Contributor

The resolve library is used to e.g. import a module as if you are in another directory.
E.g.

const resolve = require("resolve");
const tap = require(resolve.sync('tap', { basedir: otherDir }));

An alternative would be to treat it as a taint-step, but I feel it's more appropriate to treat it as a sink.

Evaluation looks fine.

@erik-krogh erik-krogh added the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Jun 4, 2021
@erik-krogh erik-krogh removed the Awaiting evaluation Do not merge yet, this PR is waiting for an evaluation to finish label Jun 6, 2021
@erik-krogh erik-krogh marked this pull request as ready for review June 6, 2021 20:10
@erik-krogh erik-krogh requested a review from a team as a code owner June 6, 2021 20:10
ResolveModuleSink() {
this = API::moduleImport("resolve").getACall().getArgument(0)
or
this = API::moduleImport("resolve").getMember("sync").getACall().getArgument(0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also add a test for this disjunct?

@erik-krogh erik-krogh changed the title JS: add the resolve library as a sink to js/path-injection JS: add the resolve library as a sink to js/path-injection Jun 8, 2021

@asgerf asgerf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍. Even if the result doesn't flow to require, it can be used to probe the existence of files on disk.

@codeql-ci codeql-ci merged commit fec3985 into github:main Jun 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants